Add retrieval-provider embedding capability#196
Merged
Conversation
Add the embedding surface of the new retrieval-provider capability: an EmbeddingProvider protocol, the EmbeddingResponse / EmbeddingUsage / EmbeddingRuntimeConfig types, and an OpenAIEmbeddingProvider reference impl posting to an OpenAI-compatible /v1/embeddings. Typed EmbeddingEvent / EmbeddingFailedEvent join the observer event union, dispatched on every embed(); the bundled OTel and Langfuse observers safely skip them for now (the embedding span and Embedding observation are a follow-up). The provider rejects malformed responses (missing, empty, or non-numeric vectors and non-permutation indices), guards a trailing /v1 on base_url, and offers a configurable readiness probe defaulting to a universal one-input embed so it works against backends without a /v1/models catalog (e.g. TEI's OpenAI surface). Conformance fixtures 001-005 pass; unit tests cover the guard, the readiness modes, and the observer safe-handling. conformance.toml keeps 0059 not-yet; the observability rendering (074-083), the cross-provider helper cleanups, and the docs are v0.16.0 follow-ups.
There was a problem hiding this comment.
Pull request overview
Adds the retrieval-provider embedding capability surface to openarmature, including a reference OpenAI-compatible embedding provider, typed embedding observer events, and conformance/unit tests to validate behavior against the spec fixtures.
Changes:
- Introduces the
openarmature.retrievalpackage withEmbeddingProvider, response/config models, and input/response validation helpers. - Adds
OpenAIEmbeddingProviderimplementingPOST /v1/embeddingsplus readiness probes (/v1/modelsand/or embed probe). - Extends the observer event union with
EmbeddingEvent/EmbeddingFailedEvent, and updates bundled OTel/Langfuse observers to safely ignore them for now; adds tests and conformance harness.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_retrieval_provider.py | Unit coverage for base_url guard, readiness probe modes, and observer skip behavior for embedding events. |
| tests/conformance/test_retrieval_provider.py | Conformance harness running retrieval-provider fixtures 001–005 against OpenAIEmbeddingProvider via httpx.MockTransport. |
| src/openarmature/retrieval/response.py | Defines EmbeddingResponse, EmbeddingUsage, and EmbeddingRuntimeConfig models. |
| src/openarmature/retrieval/providers/openai.py | Implements the OpenAI-compatible embeddings provider, request/response parsing, readiness probes, and event dispatch. |
| src/openarmature/retrieval/providers/init.py | Exposes bundled retrieval providers. |
| src/openarmature/retrieval/provider.py | Adds the EmbeddingProvider protocol and shared validators for embedding input/response invariants. |
| src/openarmature/retrieval/init.py | Public package exports for the retrieval-provider capability. |
| src/openarmature/observability/otel/observer.py | Updates OTel observer to type-union accept and explicitly ignore embedding events. |
| src/openarmature/observability/langfuse/observer.py | Updates Langfuse observer to type-union accept and explicitly ignore embedding events. |
| src/openarmature/observability/correlation.py | Widens the dispatch event type union to include embedding events (TYPE_CHECKING-only type aliasing). |
| src/openarmature/graph/observer.py | Extends ObserverEvent union with embedding events. |
| src/openarmature/graph/events.py | Adds EmbeddingEvent / EmbeddingFailedEvent dataclasses and exports them. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix the issues from the PR #196 re-review: - Build the embeddings request body extras-first so a caller's undeclared runtime-config extra cannot clobber the bound model or the input list. - Reject non-numeric vector values (JSON strings, bools) and bool usage counts strictly rather than coercing them, so a non-numeric vector is treated as malformed. - Dedup the observer event union: correlation and both observers now reference the single ObserverEvent alias instead of repeating the member list, so a new event widens the contract in one place. - Move a stray spec section ref out of a docstring into a comment and reword em-dash substitutes in docstrings. - Strengthen the tests: the Langfuse safe-handling test asserts no trace is created, and the conformance runner tolerates a fixture without a stores_response_in key instead of raising KeyError. - Refresh the observer dispatch docstrings to point at ObserverEvent rather than a hardcoded variant count.
From the PR #196 review: - _probe_models now raises ProviderInvalidResponse when the /v1/models body is not a JSON object or its data field is missing or not a list, mirroring _parse_response. A malformed catalog was being degraded to an empty list and misreported as a missing model (ProviderInvalidModel); that category is now reserved for a well-formed catalog that genuinely lacks the bound model. Covered by a new unit test. - Align the _classify_embedding_http_error docstring with the code: the catch-all maps every other status (not just 5xx) to unavailable, the same as the sibling classify_http_error.
The Observer protocol docstring example annotated its log_observer with NodeEvent | MetadataAugmentationEvent, which would be a structural conformance error now that the protocol delivers the full ObserverEvent union (embedding, tool, and provider events included). Annotate the example with ObserverEvent so copied observers type-check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the embedding surface of the new retrieval-provider capability.
retrieval/package: theEmbeddingProviderprotocol,EmbeddingResponse/EmbeddingUsage/EmbeddingRuntimeConfigtypes,input/response validators, and
OpenAIEmbeddingProvider(a referenceimplementation posting to an OpenAI-compatible
/v1/embeddings).EmbeddingEvent/EmbeddingFailedEventon the observer eventunion, dispatched on every
embed(). The bundled OTel and Langfuseobservers safely skip them for now; the embedding span and Embedding
observation are a follow-up.
Conformance fixtures 001-005 pass; the rerank fixtures (006-012) are
deferred until the
RerankProviderlands.Hardening and design
non-permutation indices) raise
provider_invalid_response./v1onbase_urlis rejected (it would double to/v1/v1).ready()takes a configurablereadiness_probe, defaulting to auniversal one-input embed so it works against backends without a
/v1/modelscatalog (such as TEI's OpenAI surface), withmodelsand
bothopt-ins.Tests
tests/conformance/test_retrieval_provider.pydrives the providerthrough
httpx.MockTransport(001-005 green).tests/unit/test_retrieval_provider.pycovers the base_url guard, thereadiness modes, and the observers' safe handling of the embedding
events.
Full conformance + unit suite green (1448 passed), whole-src pyright clean.
Follow-ups (this release)
Embedding observation, and fixtures 074-083.
event-identity snapshot, and the test MockTransport builder.